-
Notifications
You must be signed in to change notification settings - Fork 4
Un-xfail tests which needed fancy indexing; add .view
#61
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
9caf080
to
6c85cd3
Compare
6c85cd3
to
c21b892
Compare
.view
On CI this fails in
Any suggestions @honno? |
To me it looks like we need to wrap sequences as tensors as an additional step, as its saying tensors can't handle an input NumPy can In [1]: t = torch.zeros(256)
[PYFLYBY] import torch
In [2]: t[0:8] = [0. for _ in range(8)]
TypeError: can't assign a list to a torch.FloatTensor
In [3]: x = np.zeros(256)
[PYFLYBY] import numpy as np
In [4]: x[0:8] = [0. for _ in range(8)] I think this is a regression I caused by replacing >>> t[0:8] = torch.as_tensor([0. for _ in range(8)])
# no errors |
@@ -155,6 +155,13 @@ def copy(self, order="C"): | |||
tensor = self._tensor.clone() | |||
return ndarray._from_tensor_and_base(tensor, None) | |||
|
|||
def view(self, dtype): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should to the refactor where methods are implemented in terms of free functions sooner than later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes. I was holding this refactor to minimize the number of conflicts for gh-23. Now that it's in, it's about time indeed.
Something like from typing import Sequence
...
def __setitem__(...):
...
if isinstance(value, Sequence) and all(
isinstance(e, (int, float, complex)) for e in value
):
value = asarray(value).get()
... Doesn't cover nested sequence setting (I'd want to introduce a new thorough test for that—feel free to assign), but this change is easy and supports by-far the most common sequenced-value setting operation. |
How does NumPy process these nested sequences into an array? It may be best to simply copy their algorithm. |
What numpy actually does is well hidden in its C code. Maybe this test is of help: https://github.com/numpy/numpy/blob/main/numpy/core/tests/test_indexing.py#L808 |
My point is, this logic must be somewhere, in Python, C, or whichever language. We may be able to carve it out and implement it here in Python. That's what we did for plenty of functions in PrimTorch really. |
OK, this PR has run its course. |
Add a
.view
method so that we can remove several xfails.